Skip to content

fix(flags): explain silent cohort non-match in flag evaluation - #83866

Merged
trunk-io[bot] merged 4 commits into
masterfrom
posthog-self-driving/fixflags-stop-cohort-conditions-335dbb
Aug 25, 2026
Merged

trunk-io[bot] merged 4 commits into
masterfrom
posthog-self-driving/fixflags-stop-cohort-conditions-335dbb

Conversation

@posthog

@posthog posthog Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Problem

  • A person rolling a flag out to a cohort can reach nobody while the cohort's member list shows the audience present, and nothing explains why.
  • The flag evaluator cannot fully resolve a behavioral or realtime cohort: behavioral and lifecycle leaves are skipped on the dynamic path, and a realtime-routed cohort is treated as a non-member when realtime evaluation is off.
  • The result was a bare no_condition_match, indistinguishable from a genuine negative, so the disagreement between flag evaluation and the cohort's precomputed member list stayed invisible.
  • This does not change which users a flag reaches. It makes an already-happening non-match legible; the underlying realtime-cohort membership work is tracked separately.

Changes

  • Add FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated, returned from is_condition_match when a targeted cohort with a behavioral or lifecycle condition decides the non-match.
  • It serializes as no_condition_match for backward compatibility and carries an enriched description, mirroring the existing NoConditionMatchGroupsNotEvaluated reason.
  • A person-property-only cohort that genuinely does not match stays NoConditionMatch, so trustworthy negatives are not relabeled.
  • Forward the description through the evaluation_reasons and flag-test endpoints.
  • Surface the explanation in the person profile flags tab (info tooltip on the non-match) and the flag tester (info banner).
Cohort a flag targets Non-match reason
Person-property only no_condition_match (unchanged)
Behavioral or lifecycle leaf no_condition_match + description explaining the divergence

How did you test this code?

  • Added two Rust matcher tests: a behavioral cohort non-match reports the new reason; a person-property cohort non-match stays NoConditionMatch.
  • Ran the feature-flags lib suite locally against a Postgres test database: the two new tests pass, and the cohort, realtime, and reason clusters (111 tests) pass with no regressions.
  • I could not run the ClickHouse-backed Django test suite in this environment (the prebaked dev stack's ClickHouse container fails to start on a stale config bind-mount), so the Python endpoint tests were not run locally and are left to CI.
  • Regenerated OpenAPI types (hogli build:openapi); the only generated changes are the added reason_description field.

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Fully autonomous

  • Authored by Claude Code (PostHog Desktop) from an inbox report about cohort-targeted flags reaching nobody.
  • Skills invoked: /improving-drf-endpoints, /writing-tests, /writing-user-facing-copy, /writing-code-comments, /writing-pr-descriptions.
  • Scope decision: much of the report's Rust hardening (behavioral-leaf tolerance, unknown-group-type errors, malformed-filter recording) already exists, and the evaluate_single_cohort deserialize-failure path is guarded upstream by extract_dependencies. The remaining live symptom is the silent non-match for behavioral and realtime cohorts. This PR makes that non-match explainable without changing match behavior, deliberately staying clear of the realtime-cohort membership work tracked in other issues.
  • No customer data, session material, or private context is included; the cohort scenarios in tests use invented person properties.

Created with PostHog Desktop from this inbox report.

A flag targeted at a behavioral or realtime cohort could report a bare
no-match, which reads as a correct negative even when the cohort's member
list shows the person as present. The evaluator cannot fully resolve those
cohorts (behavioral or lifecycle leaves are skipped on the dynamic path, and
a realtime-routed cohort is a non-member when realtime evaluation is off).

Add a distinct FeatureFlagMatchReason, NoConditionMatchCohortNotEvaluated,
returned when a targeted cohort with a behavioral or lifecycle condition
decides the non-match. It still serializes as no_condition_match for
backward compatibility, and carries an enriched description, mirroring the
existing NoConditionMatchGroupsNotEvaluated pattern.

Forward that description through the evaluation_reasons and flag-test
endpoints, and surface it in the person profile flags tab and the flag
tester so the non-match is explained rather than looking correct.

Generated-By: PostHog Desktop
Task-Id: 5274be96-cfc3-40cb-b2ee-db0a190ff6bf
@trunk-io

trunk-io Bot commented Aug 17, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

⚠️ Bundle size — 🔺 +64 B (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 68.50 MiB · 🔺 +64 B (+0.0%)

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

✅ Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.36 MiB · 22 files no change ███░░░░░░░ 30.2% of 4.51 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.77 MiB · 3,235 files no change █████████░ 90.2% of 9.71 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
789 B src/scenes/ChunkLoadErrorBoundary.tsx
762 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
306.5 KiB ../node_modules/.pnpm/posthog-js@1.418.17/node_modules/posthog-js/dist/rrweb.js
267.7 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
258.5 KiB ../node_modules/.pnpm/posthog-js@1.418.17/node_modules/posthog-js/dist/module.js
246.0 KiB src/taxonomy/core-filter-definitions-by-group.json
154.2 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
104.6 KiB src/lib/api.ts
95.1 KiB ../packages/quill/packages/quill/dist/index.js
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

✅ Toolbar bundle — eager 2.25 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.25 MiB · 17 files no change ████░░░░░░ 39.3% of 5.72 MiB
Deferred (lazy) 2.09 MiB · 33 files no change n/a — loads on demand
Loader dist/toolbar.js 1.1 KiB no change █░░░░░░░░░ 5.8% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
745.7 KiB dist/toolbar/toolbar-app-XJGYQ44O.css
582.9 KiB dist/toolbar/chunk-chunk-YRYIO5AK.js
484.6 KiB dist/toolbar/chunk-chunk-7TYJBGPX.js
133.8 KiB dist/toolbar/chunk-chunk-HUJ6YVZZ.js
131.8 KiB dist/toolbar/chunk-chunk-T5KY5WYR.js
71.2 KiB dist/toolbar/toolbar-app-2USHGT3U.js
69.0 KiB dist/toolbar/chunk-chunk-27JL52RE.js
35.6 KiB dist/toolbar/chunk-chunk-5I6DF2DA.js
20.9 KiB dist/toolbar/chunk-chunk-WRHMR2RW.js
12.2 KiB dist/toolbar/chunk-chunk-PIK3PADE.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — 🔺 +6.6 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1460.19 MiB · 🔺 +6.6 KiB (+0.0%)

⚠️ Playwright — 1 flaky

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

  • Inline editing insight title via compact card popover (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!

⚠️ Backend snapshots — 1 updated (1 modified, 0 added, 0 deleted)

Query snapshots: Backend query snapshots updated

Changes: 1 snapshots (1 modified, 0 added, 0 deleted)

What this means:

  • Query snapshots have been automatically updated to match current output
  • These changes reflect modifications to database queries or schema

Next steps:

  • Review the query changes to ensure they're intentional
  • If unexpected, investigate what caused the query to change

Review snapshot changes →

✅ ClickHouse migration SQL — none

No ClickHouse migrations in the latest push.

@trunk-io

trunk-io Bot commented Aug 17, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

Shorten the multi-line comments added for the cohort non-match reason to at
most two lines each, keeping the why and dropping the restated detail.

Generated-By: PostHog Desktop
Task-Id: da3a77ad-8157-4394-8389-4f980e248485
@patricio-posthog
patricio-posthog marked this pull request as ready for review August 25, 2026 18:40
@github-project-automation github-project-automation Bot moved this from Todo to Approved in Feature Flags Aug 25, 2026
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested review from a team August 25, 2026 18:41
@posthog-project-board-bot posthog-project-board-bot Bot moved this from Approved to In Review in Feature Flags Aug 25, 2026
@posthog

posthog Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

👋 Visual changes detected for this PR.

Review and approve in PostHog Visual Review

If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix.

@trunk-io
trunk-io Bot merged commit d163cb4 into master Aug 25, 2026
258 checks passed
@trunk-io
trunk-io Bot deleted the posthog-self-driving/fixflags-stop-cohort-conditions-335dbb branch August 25, 2026 20:34
@github-project-automation github-project-automation Bot moved this from In Review to Done in Feature Flags Aug 25, 2026
@deployment-status-posthog

deployment-status-posthog Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-08-25 21:06 UTC Run
prod-us ✅ Deployed 2026-08-25 21:20 UTC Run
prod-eu ✅ Deployed 2026-08-25 21:24 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant